-
-
Notifications
You must be signed in to change notification settings - Fork 250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fast rendering #1835
Add fast rendering #1835
Conversation
Can this replace the implementation of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Had a look through the code, couldn't find anything really standing out. Just some minor recommendations for performance
Super clean! I wonder if this also solves #1794? |
acd35cc
to
8dd0c1e
Compare
f960102
to
aae2c73
Compare
68bfdc7
to
2ee4d21
Compare
federation/src/test/scala/caliban/federation/FederationV1Spec.scala
Outdated
Show resolved
Hide resolved
2ea4b03
to
a1ba636
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Introduces a
Renderer
type and aDocumentRenderer
implementation which generalizes the the rendering of aDocument
so that it can work with both query and schema documents (the inverted parser).Running the benchmarks:
The above benchmarks show a 2x improvement over the base rendering (calling
GraphQL#render
) when using.toDocument
, however, if we cache the document (something I added on this PR) we see an 8x improvement on rendering.This should help with use cases that require the document to be re-serialized (e.g. federation, stitching, etc.)
TODO: